Skip to content

ENH: fill_value argument for shift #15486 #24128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 113 commits into from
Dec 26, 2018
Merged

Conversation

ahcub
Copy link
Contributor

@ahcub ahcub commented Dec 6, 2018

@pep8speaks
Copy link

pep8speaks commented Dec 6, 2018

Hello @ahcub! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on December 26, 2018 at 00:17 Hours UTC

@codecov
Copy link

codecov bot commented Dec 6, 2018

Codecov Report

Merging #24128 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #24128      +/-   ##
==========================================
+ Coverage   92.29%    92.3%   +0.01%     
==========================================
  Files         162      163       +1     
  Lines       51832    51978     +146     
==========================================
+ Hits        47839    47980     +141     
- Misses       3993     3998       +5
Flag Coverage Δ
#multiple 90.71% <100%> (+0.01%) ⬆️
#single 42.99% <23.33%> (+0.01%) ⬆️
Impacted Files Coverage Δ
pandas/core/series.py 93.72% <ø> (+0.01%) ⬆️
pandas/core/arrays/base.py 97.59% <100%> (+0.17%) ⬆️
pandas/core/frame.py 96.91% <100%> (ø) ⬆️
pandas/core/generic.py 96.62% <100%> (-0.04%) ⬇️
pandas/core/arrays/period.py 98.48% <100%> (-0.02%) ⬇️
pandas/core/internals/blocks.py 93.81% <100%> (ø) ⬆️
pandas/core/groupby/groupby.py 96.8% <100%> (+0.15%) ⬆️
pandas/core/arrays/categorical.py 95.34% <100%> (+0.02%) ⬆️
pandas/core/arrays/sparse.py 92.1% <100%> (+0.01%) ⬆️
pandas/core/arrays/datetimes.py 97.6% <0%> (-0.64%) ⬇️
... and 25 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f6cf7d9...a03cbf5. Read the comment docs.

@codecov
Copy link

codecov bot commented Dec 6, 2018

Codecov Report

Merging #24128 into master will decrease coverage by 49.17%.
The diff coverage is 58.33%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #24128       +/-   ##
===========================================
- Coverage    92.2%   43.03%   -49.18%     
===========================================
  Files         162      162               
  Lines       51701    51701               
===========================================
- Hits        47672    22247    -25425     
- Misses       4029    29454    +25425
Flag Coverage Δ
#multiple ?
#single 43.03% <58.33%> (ø) ⬆️
Impacted Files Coverage Δ
pandas/core/frame.py 35.88% <100%> (-61.03%) ⬇️
pandas/core/internals/blocks.py 52.93% <50%> (-40.79%) ⬇️
pandas/core/series.py 49.32% <50%> (-44.38%) ⬇️
pandas/core/generic.py 39.65% <50%> (-57%) ⬇️
pandas/core/arrays/datetimelike.py 48.74% <66.66%> (-47.61%) ⬇️
pandas/io/formats/latex.py 0% <0%> (-100%) ⬇️
pandas/core/categorical.py 0% <0%> (-100%) ⬇️
pandas/io/sas/sas_constants.py 0% <0%> (-100%) ⬇️
pandas/tseries/plotting.py 0% <0%> (-100%) ⬇️
pandas/tseries/converter.py 0% <0%> (-100%) ⬇️
... and 124 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b841374...8b5cedb. Read the comment docs.

@ahcub ahcub changed the title ENH: fill_value argument for shift ENH: fill_value argument for shift #15486 Dec 6, 2018
@@ -320,6 +320,19 @@ def test_shift_categorical(self):
xp = DataFrame({'one': s1.shift(1), 'two': s2.shift(1)})
assert_frame_equal(rs, xp)

def test_shift_fill_value(self):
df = DataFrame(np.random.randnint(5), index=date_range('1/1/2000',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add the gh number as a comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@jreback jreback added Enhancement Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels Dec 6, 2018
Copy link
Contributor

@TomAugspurger TomAugspurger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably need to add to Categorical, SparseArray. What about groupby?

@ahcub
Copy link
Contributor Author

ahcub commented Dec 7, 2018

added fill_value to shift implementations across different classes, according to the failures from tests

@ahcub
Copy link
Contributor Author

ahcub commented Dec 8, 2018

@jreback I made all the changes, except the Index subclasses, because I think Index doesn't need fill_value. it does the shift correctly already with the freq specified
can you review?

@mroeschke
Copy link
Member

One last linting error as well:

2018-12-25T18:31:36.1417763Z ERROR: /home/vsts/work/1/s/pandas/tests/arrays/sparse/test_array.py Imports are incorrectly sorted.
2018-12-25T18:31:36.1417924Z ERROR: /home/vsts/work/1/s/pandas/core/arrays/base.py Imports are incorrectly sorted.

@ahcub
Copy link
Contributor Author

ahcub commented Dec 25, 2018

used isort to fix those, hopefully no issues this time 🙏

@ahcub
Copy link
Contributor Author

ahcub commented Dec 25, 2018

just added the whatsnew note, not sure if it is ok
all the other checks were green

@ahcub
Copy link
Contributor Author

ahcub commented Dec 25, 2018

all green @jreback

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 tiny corrections. ping on green and will merge. thanks for being so responsive.

@@ -31,6 +31,7 @@ New features
- :func:`read_feather` now accepts ``columns`` as an argument, allowing the user to specify which columns should be read. (:issue:`24025`)
- :func:`DataFrame.to_html` now accepts ``render_links`` as an argument, allowing the user to generate HTML with links to any URLs that appear in the DataFrame.
See the :ref:`section on writing HTML <io.html>` in the IO docs for example usage. (:issue:`2679`)
- :meth:`DataFrame.shift` :meth:`Series.shift`, :meth:`ExtensionArray.shift`, :meth:`SparseArray.shift`, :meth:`Period.shift`, :meth:`GroupBy.shift`, :meth:`Categorical.shift`, :meth:`NDFrame.shift` and :meth:`Block.shift` now accept fill_value as an argument, allowing the user to specify a value which will be used instead of NA/NaT in the empty periods
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you use double backticks on fill_value and add a reference to the issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

@@ -9,6 +9,8 @@
from pandas.compat import range
import pandas.util._test_decorators as td

from pandas.core.dtypes.missing import isna
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in tests just
from pandas import isna

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed

@jreback jreback added this to the 0.24.0 milestone Dec 25, 2018
@jreback
Copy link
Contributor

jreback commented Dec 26, 2018

@ahcub lgtm. ping on green.

@ahcub
Copy link
Contributor Author

ahcub commented Dec 26, 2018

all green @jreback

@jreback jreback merged commit bf31c04 into pandas-dev:master Dec 26, 2018
@jreback
Copy link
Contributor

jreback commented Dec 26, 2018

thanks @ahcub

this was a pretty involved change, thanks for sticking with it!

@ahcub
Copy link
Contributor Author

ahcub commented Dec 26, 2018

thank you for all the support!

thoo added a commit to thoo/pandas that referenced this pull request Dec 28, 2018
* upstream/master: (26 commits)
  DOC: Fixing doc upload (no such remote origin) (pandas-dev#24459)
  BLD: for C extension builds on mac, target macOS 10.9 where possible (pandas-dev#24274)
  POC: _eadata (pandas-dev#24394)
  DOC: Correct location (pandas-dev#24456)
  CI: Moving CircleCI build to Travis (pandas-dev#24449)
  BUG: Infer compression by default in read_fwf() (pandas-dev#22200)
  DOC: Fix minor typo in whatsnew (pandas-dev#24453)
  DOC: Add dateutil to intersphinx pandas-dev#24437 (pandas-dev#24443)
  DOC: Adding links to offset classes in timeseries.rst (pandas-dev#24448)
  DOC: Adding offsets to API ref (pandas-dev#24446)
  DOC: fix flake8 issue in groupby.rst (pandas-dev#24363)
  DOC: Fixing more doc warnings (pandas-dev#24438)
  API: Simplify repeat signature (pandas-dev#24447)
  BUG: to_datetime(Timestamp, utc=True) localizes to UTC (pandas-dev#24441)
  CLN: Cython Py2/3 Compatible Imports (pandas-dev#23940)
  DOC: Fixing more doc warnings (pandas-dev#24431)
  DOC: Removing old release.rst (pandas-dev#24427)
  BUG-24408 Series.dt does not maintain own copy of index (pandas-dev#24426)
  DOC: Fixing several doc warnings (pandas-dev#24430)
  ENH: fill_value argument for shift pandas-dev#15486 (pandas-dev#24128)
  ...
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate Reshaping Concat, Merge/Join, Stack/Unstack, Explode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ENH: fill_value argument for shift
5 participants